Fix regular expression parsing #3244
Conversation
Seldaek
force-pushed
the
bug11323
branch
3 times, most recently
from
f3b0770 to
56a35df
Compare
| @@ -372,12 +372,13 @@ private function parseGroups(string $regex): ?array | |||
| { | |||
| if (self::$parser === null) { | |||
| /** @throws void */ | |||
| self::$parser = Llk::load(new Read('hoa://Library/Regex/Grammar.pp')); | |||
| self::$parser = Llk::load(new Read(__DIR__ . '/../../../resources/RegexGrammar.pp')); | |||
There was a problem hiding this comment.
@ondrejmirtes Are we ok loading the file from here instead? Not sure what this does in terms of licensing, nor if there's a better place to put it. If so I'll delete the patch file and drop the hoa/regex requirement.
There was a problem hiding this comment.
I am not sure this path will work in the packaged phpstan.phar - while I agree that having the RegexGrammar.pp in the phpstan-src repo might ease the contribution experience - as the patching is really cumbersome.
There was a problem hiding this comment.
I completely agree moving from the patching as the patching is very hard to understand and the changes needed are more and more complex.
There was a problem hiding this comment.
Ok I got rid of it, we'll see what @ondrejmirtes says :)
There was a problem hiding this comment.
I propose to add the whole and unpatched RegexGrammar.pp as first/separate commit, then the patched/actual grammar and then the changes from this PR. This will hugely improve the review and git history.
|
This pull request has been marked as ready for review. |
|
@staabm please review the last |
|
|
||
|
|
||
| // Skip. | ||
| %skip nl \n |
There was a problem hiding this comment.
\n can be in pcre class as well
There was a problem hiding this comment.
Yes I am unsure why this is here, I haven't experimented much, but it is rare to have actual newlines in regexes as they're usually escaped as \n, while this here matches a real newline.
I'll create a new issue for this to look at it later, together with proper support for the x modifier, because it's very much related. Edit: phpstan/phpstan#11360
- Skip regex parsing if the pattern is not valid, but report errors then if parsing fails - Fix support for internal options to include more options, and fix []] (i.e. class end delimiter as first character is not a literal) not parsing correctly - Add a couple missing quantifiers in getQuantificationRange - Handle more literal values and fix escaped ones to be unescaped
|
I cherry-picked the grammar in the repo directly to 1.11.x, rebased this PR, and merged it. Thanks! :) |
I moved the grammar file into the repo instead of the hoa regex one with patch, that makes it a lot easier to work with.
This PR fixes parsing of complex character classes which was really not well supported in hoa/regex, along with a bunch of other smaller issues.